refactor: move cli/templates to a top-level templates/ directory - #3596
Merged
Conversation
Templates stopped being a CLI concern. `src/` already reaches into `cli/templates/integrations/` from five test files, and veryfront-api is moving to read the same generated manifest, so keeping the directory under the CLI package was a layering violation on both sides. The move follows the repo's existing precedent for separately-shipped things (`extensions/ext-*`): a top-level sibling. All four subdirectories move together — `ai-rules/`, `features/`, `files/`, `integrations/` — plus the loaders and the generated manifest, so the package has one root. Nothing about template content changes. Every one of the 926 files moves with `git mv` and zero content edits; the regenerated manifest is byte-identical to the committed one; and all seven starter templates scaffolded through the real CLI (plus an ai-agent + github,slack integration build) diff clean against the same scaffold taken from origin/main. The lint exclusions that keep template files out of the source lint gate move with it. `deno lint` still reports 4917 files and `deno fmt --check` still reports 4763, exactly as before — the templates are excluded, not newly linted. Scan roots that enumerated `src` and `cli` (coverage sharding, sanitizer/skipped-test/typecheck baselines, cwd-relative test reads, duplicate-function detection, the unit-test file globs) gain `templates` so the loaders and their tests keep the coverage they had inside `cli/`. Published layout changes: the manifest ships at `esm/templates/manifest.js` instead of `esm/cli/templates/manifest.js`. Consumers resolving it by relative URL off `veryfront/cli` (`esm/cli/main.js`) need `../templates/manifest.js` where they used `./templates/manifest.js`. Refs veryfront-issue-inbox#477
|
Important Review skippedToo many files! This PR contains 757 files, which is 457 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (211)
📒 Files selected for processing (757)
You can disable this status message by setting the Comment |
Three lint tools carried an explicit `cli/templates/` exclusion because the directory sat under a scanned root. At the top level it no longer does, so rewriting the string to `templates/` would have left code that reads as a live guard while never firing. - `audit-core-deps.ts` and `ban-zod-imports.ts` return early for anything outside `src/` and `cli/`, which now covers templates. The clause is replaced by a comment saying why no clause is needed; the existing tests still pin the behaviour. - `find-duplicate-functions.ts` keeps `templates/` in DEFAULT_EXCLUDES, which stays live for explicit `deno task dupes templates` invocations, but drops it from DEFAULT_PATHS where it was scanned and then excluded in the same breath. No behaviour change: dupes still reports 234 groups, and both audits still pass with the same output.
Contributor
Author
CI status: all 9 required checks pass; one non-required check needs a human action
The three alerts are not new findings. They are the same three alerts, on the same rules, in the same files, at the same lines — already adjudicated and dismissed on
All three were dismissed by @ariskemper as false positive with the comment:
That rationale is about the file's role, not its path, so it applies verbatim after the move. The code itself is provably untouched: I deliberately did not re-dismiss them. Re-applying a security adjudication is the repo owner's call, not something to automate on their behalf. To restore the previous state: for n in 276 277 278; do
gh api -X PATCH "repos/veryfront/veryfront-code/code-scanning/alerts/$n" \
-f state=dismissed -f dismissed_reason='false positive' \
-f dismissed_comment='Scaffold template file — copied to user projects, not runtime code'
doneWorth deciding separately: whether scaffold template trees should be in CodeQL's scope at all. If not, a |
kojiwakayama
added a commit
that referenced
this pull request
Aug 11, 2026
…olds it Rebased onto the `cli/templates/` -> `templates/` move (#3596). `./scaffold` keeps its name - it names the capability, not the directory - and now points at `./templates/scaffold.ts`. The second export this branch declared, `./cli/templates/manifest`, is gone. The manifest is data; `materializeScaffold()` is the behaviour. A consumer reading the manifest has to reimplement package.json generation, AGENTS.md injection and .gitignore for itself - a second scaffolder fed from one data source, which is issue #475 one level down. veryfront-api will call `materializeScaffold()`, and nothing in this repository or its siblings reads the raw manifest, so the export had no consumer to keep. Nothing is published yet, so no name was burned. The enforcement stays, retargeted: - `templates/scaffold-export.test.ts` fails if `./scaffold` stops being declared, if it points at a file that no longer exists - the exact failure the directory move would have caused - or if the module stops exporting the behaviour a caller imports it for. - `npm-install-smoke.sh` step 6 imports the bare specifier from a clean-room install and materializes a project through it, so Node resolves it against the published `exports` map. The deep `node_modules/veryfront/esm/...` paths the other steps use bypass that map, so this is the only step that can catch the export going missing; without the entry it fails with ERR_PACKAGE_PATH_NOT_EXPORTED. `scaffold-quality.test.ts` reached the repo config through `../../deno.json`, which resolves above the repository now that the file sits one level higher; every template's type-check step failed on the missing config until it was repointed at `../deno.json`.
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves
cli/templates/to a top-leveltemplates/directory. This PR is only the move — nopackage.json, no publishing config, no extraction.Why
The directory stopped being a CLI concern:
src/already reaches intocli/templates/integrations/from five test files (src/oauth/providers/{atlassian,google,microsoft,common}.test.ts,src/integrations/_data.test.ts) — a layering violation today.The repo's own precedent for a separately-published thing is a top-level sibling (
extensions/ext-*), sotemplates/follows that. Preparation for veryfront-issue-inbox#477 (@veryfront/templates).All four subdirectories move together —
ai-rules/,features/,files/,integrations/— plus the loaders (index.ts,loader.ts,types.ts,feature-loader.ts,integration-loader*.ts) and the generatedmanifest.json, so the package has a single root. Every file moved withgit mv, so history follows.Acceptance: a scaffolded project is byte-identical
Two independent proofs, both run before and after the move:
1. Real CLI scaffolding.
origin/mainwas extracted to a scratch tree withgit archive, and all seven starter templates plus anai-agent --integrations github,slackbuild were scaffolded withveryfront init --skip-install --skip-env-promptfrom each tree:CLI stdout was identical too.
2. Loader-API dump. A 443-file dump of everything the templates package can produce — every starter tree, every integration tree raw + namespaced + merged, integration base files, all 204 connector configs, all ai-rules, all feature configs, and the registry ordering — diffed clean.
git diff --stat -Mreports 926 files changed, 0 insertions(+), 0 deletions(-) for the moved tree: a pure rename with zero content edits.Manifest
The manifest is generated (
scripts/build/generate-templates-manifest.ts). Regenerated at its new location it is byte-identical to the version committed onorigin/main:Template file paths inside the manifest are relative to each template root, so they do not move. Same 48 templates, 464 files.
The lint exclusions still exclude
deno.jsonexcludestemplates/files/andtemplates/integrations/so template sources are not linted as framework source. Becauselint.include/fmt.includeenumerate roots explicitly,templates/**was added there too — otherwise the loaders would have silently dropped out of both gates. Verified by comparing file counts against theorigin/maintree:deno lintdeno fmt --check src/ cli/ react/ [templates/]Not one more file, not one fewer.
Scan roots that enumerated
srcandcliSeveral tools hard-code the roots they walk. Left alone, the loaders and their four test files would have quietly left every one of them.
templateswas added to:scripts/test/coverage-ci.ts(UNIT_COVERAGE_ROOTS) — coverage shardingscripts/lint/check-sanitizer-baseline.ts,check-skipped-tests-baseline.ts,ban-test-only.ts(SCAN_ROOTS)scripts/lint/audit-cwd-relative-test-reads.ts(SCAN_ROOTS)scripts/lint/check-test-typecheck-baseline.ts(listTestFiles)scripts/lint/find-duplicate-functions.ts(DEFAULT_PATHS)deno.jsontest:unit:parallelandtest:coverage:unit(find src cli templates)Published layout change (breaks veryfront-api#4323's assumption)
Verified by running
deno task build:npm:veryfront/esm/cli/templates/manifest.jsveryfront/esm/templates/manifest.js(npm/esm/cli/templates/no longer exists)templatesis not in the packageexportsmap, so the only way to reach the manifest is a relative URL off an exported entry. veryfront-api#4323 resolves it by hopping offveryfront/cli, which resolves toesm/cli/main.js. That hop must change:(The in-repo consumer,
cli/commands/install/registry.ts, now emitsimport manifest from "../../../templates/manifest.js"in the built package — confirmed innpm/esm/.) Not fixed here — #4323 needs the one-line update.Other paths updated
deno.json(exclude, lint/fmt include, fmt + test tasks),.gitignore,socket.yml,.github/actions/setup-deno/action.yml,scripts/build/generate-templates-manifest.ts,scripts/build/generate-integrations-module.ts,scripts/build/npm-package-metadata.test.ts,scripts/lint/{audit-core-deps,ban-zod-imports,find-duplicate-functions}.ts(+ tests),scripts/test/npm-install-smoke.sh,cli/commands/install/registry.ts, all relative imports acrosscli/,src/,tests/, andcli/README.md.grep -rn "cli/templates"over the repo returns nothing.Verified locally
fmt:check,lint,typecheck,generate:manifests:check,docs:api-reference:check,docs:validate, everylint:*gate,build:npm, and the fulltest:unitsuite (via the pre-push hook) all pass.Refs veryfront-issue-inbox#477